Repository navigation
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Add NSIS packaging, signing and timestamp verification, source and artifact inventories, and explicit unsigned builds for development and CI. Protect installation changes with application lifetime locks, inventoried file ownership, staged replacement, and recoverable rollback. Preserve user state and launch from Finish with the desktop user's token and environment. Setuptools silently pruned the tracked build/windows directory from source archives because it shared the generated build directory. Move generated files to .python-build and check the Windows builder's presence in CI. Validated on Windows: 727 tests passed, 5 skipped; Ruff, unsigned NSIS and portable builds, frozen smoke checks, wheel/sdist builds, isolated wheel installation, and final inventories/checksums passed. Hardware signing and clean-machine interactive installation remain unqualified.
owenpkent
left a comment
There was a problem hiding this comment.
Reviewed the installation transaction, ownership/reparse checks, installed-instance lock, upgrade recovery, uninstall boundaries, and build/signing integration against this PR's base. A fresh-install rollback defect remains, detailed inline.
A controlled reproduction used only a synthetic payload in workspace scratch and injected a failure at the installation-manifest write. No actual application installation, signing, elevation, or clean-machine qualification was performed. The observed retry failure is independent of those remaining release gates.
Rollback removed the files it had promoted and left the directories it had made to promote them into. After a failure writing the installation manifest a synthetic first install rolled back to ['.offloader-install.lock', '_internal'], and the immediate retry failed with "refusing to install into a nonempty unowned directory", because _assert_fresh_target sees that leftover directory and no ownership manifest. So a transient first-install failure could not be retried through the installer at all, even though recovery reported no outstanding transaction. The transaction now records each directory it creates on the way to a promotion, before creating it - a crash in between leaves the marker naming a directory that is not there, which pruning tolerates, rather than a directory nothing knows about, which it cannot fix. Rollback prunes exactly those, deepest first, and only while they are still empty. A directory that already existed was never recorded and is not touched; one holding a file that arrived from somewhere else is left with it. Pruning runs last, after an upgrade has restored its old files, because those refill the same directories. Writing the regression turned up a second way the same failure wedged the install, in the same handler and with the same cause, so it is fixed here too. The manifest write is what commits an update and it is atomic: either the new inventory landed or the previous one is still there. Recovery treated the second as "manifest does not match a committing update" and raised - and every later install and uninstall recovers first, so a failed upgrade left an installation that no maintenance operation could get past. Both outcomes now mean it did not commit, and roll back. Three regressions: a rolled-back first install retried successfully, a created directory holding somebody else's file kept along with it, and a failed upgrade keeping the directories it did not create and still being upgradable and uninstallable afterwards. The first and third fail without the fix.
owenpkent
left a comment
There was a problem hiding this comment.
Follow-up review of 8d91ede, concentrating on rollback and recovery after a failed installation-manifest write.
The transaction now records newly created parent directories before creating them, removes them during rollback only while empty, and keeps pre-existing directories or directories containing somebody else's file. Recovery also recognizes a surviving old manifest during the committing phase as an uncommitted upgrade, allowing restoration instead of permanently wedging later maintenance.
Validation: all 3 new rollback regressions passed against disposable payloads on Linux: retrying a first install, preserving an unrelated file in a created directory, and recovering a failed upgrade before another install/uninstall. No additional finding in these corrections. Windows NSIS, elevation, Authenticode, and crash behavior on physical storage remain outside this local validation.
owenpkent
left a comment
There was a problem hiding this comment.
One new P2 finding: an existing registered installation location overrides an explicit NSIS /D= target; the native reproduction and fix direction are inline.
Reviewed installation ownership/rollback/recovery, shared-instance locking, entry points, artifact/version metadata, builder/signing order, installer rendering and CI integration. The earlier fresh-install rollback correction remains present.
Validation: 80 focused installation/build/artifact/lock/launch/signing tests passed in a separate pinned source export; two symlink cases skipped and one Git-source-identity case was deselected because that export has no .git metadata. A silent, unelevated NSIS 3.12 probe confirmed that /D= is consumed before .onInit and the current fallback overwrites the selected target. The probe only wrote a scratch log. Current-head CI has successful matrix/build/frozen checks; the diff check passes.
Limits: no new complete freeze, hardware-key signing, registry mutation, real installation, UAC/alternate-credential run or clean-machine qualification. Python render tests alone do not establish native installer argument behavior.
Pinned scope: declared base 67fac2fc137a48dae517ca6e194ba3e2e9498862, merge base 67fac2fc137a48dae517ca6e194ba3e2e9498862, head 6eda863bbc81630be6fe84f59d31af694ceb3500. Existing discussion and current-head checks were reviewed; no threads were resolved and no formal approval or merge is being issued.
…ation NSIS applies /D= before .onInit and strips it from $CMDLINE, so the GetOptions check never saw it and the registered InstallLocation always overwrote the target NSIS had already placed in $INSTDIR. After registering installation A, setup.exe /S /D=B updated A instead of B. .onInit now consults the registry only when $INSTDIR still holds the compiled default. The regression compiles a probe from the real rendered script with makensis, substitutes a fixed previous location for the registry read, and runs it unelevated with and without /D=, asserting the target it resolved.
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md # CONTRIBUTING.md # README.md
# Conflicts: # CONTRIBUTING.md # README.md
What this changes
Offloader can now be built as a Windows x64 desktop/CLI bundle and an NSIS installer for machines without Python. A single version source supplies package metadata, runtime/report versions, and executable/installer versions. Runtime and packaging dependencies are pinned.
The build signs by default with the existing OK Studio certificate, verifies trusted publishers and timestamp coverage, signs the generated uninstaller during NSIS compilation, and signs the setup executable afterward.
--no-signproduces development and CI artifacts without accessing the key. Source/file inventories reject stale--skip-buildbundles;--verify-onlychecks existing outputs without the private key, and final checksums cover the installer, portable ZIP, and inventory.Installed GUI and CLI processes hold lifetime locks that block maintenance while any instance is open. Maintenance blocks new launches, stages and checks replacements, preserves old owned files for rollback, recovers interrupted operations, and removes only inventoried files. Settings, history, and unrelated files are preserved. The wizard includes shortcut choices, silent installation, and an optional Finish launch using the desktop user's token and environment, with no elevated fallback.
CI builds unsigned installers and portable bundles, runs frozen smoke checks, and checks that the source archive includes the Windows builder. Setuptools output moves to
.python-buildbecause its default cleanup otherwise omitted the trackedbuild/windowsdirectory. The earlier file-control test fixes remove disk-speed races without changing production copy or verification behavior.Stacked on #6; targets
timeline-offload. Timeline import remains source-only in this bundle. No release is published and no hardware-key signing was performed.What you verified
Hardware-key signing, the actual NSIS install lifecycle under UAC, alternate administrator credentials and desktop launch, independent clean-machine/real-storage qualification, and the complete third-party license inventory/SBOM remain pending. Hosted CI for this update must be evaluated separately from the local results above.
Checklist
Review follow-ups (2026-10-11)
/D=before.onInitand strips it from$CMDLINE, so the old GetOptions check never saw it and the registered InstallLocation always won..onInitnow consults the registry only when$INSTDIRstill holds the compiled default. A regression compiles a probe from the rendered script with makensis and runs it unelevated with and without/D=. Known limit: an explicit/D=equal to the default location cannot be told apart from none.